Did you use the debugging aids to visualize the computed bounding sphere, texture coordinates, and normal/binormal/tangents?

Don't forget to update the tutorial. Even before we merge is fine; the tutorial is still a draft and I expect we'll merge this quickly.

The Sandcastle example is pretty awesome.

Looks like the triangle winding order is wrong sometimes.

I made a small change to the Sandcastle example:

    positions = ellipsoid.cartographicArrayToCartesianArray([
         Cesium.Cartographic.fromDegrees(-90.0, 10.0, 100000.0),
         Cesium.Cartographic.fromDegrees(-95.0, 15.0, 100000.0),
         Cesium.Cartographic.fromDegrees(-100.0, 15.0, 100000.0)
     ]);

In general, I would expect that the 2D origin in the plane of the polygon would be centered along the polyline, and then the height argument adjusts from there.

We should take into account the height of each position in polylinePositions. Given this, I'm not sure that we need the height property.

Looks like the triangle winding order is wrong sometimes.

The missing triangles is related to Issue #1093. If you switch the order of the geometries in the geometryInstances array parameter passed to the primitive, the orange one will be missing triangles instead.

Some of the shading can be jarring because we are using averaged vertex normals. For the wall and other primitives (including the corridor), we duplicate the vertex so we can have better normals. We'll need to do that here for the corridor primitive to use it. We should just be able to post-process the polygon (make a copy!) before extruding it. I would do it based on a heuristic of the angle between adjacent edges so we only duplicate vertices that need to be. This can be a generic function in PolygonPipeline so we can use it for the wall.

With beveled corners:

@pjcozzi I am duplicating the points:

With single points the bevel looks all rounded and shiny:

Looks like the triangle winding order is wrong sometimes.

The missing triangles is related to Issue #1093. If you switch the order of the geometries in the geometryInstances array parameter passed to the primitive, the orange one will be missing triangles instead.

CC @bagnell

@pjcozzi I am duplicating the points...

OK.

@bagnell I fixed most of the things you mentioned, but I think I'll need help fixing the texture coordinates. After that, hopefully the tangents/binormals will better. Currently I'm using GeometryPipeline.computeBinormalAndTangent to compute them.

I'm still seeing an issue similar to what @pjcozzi was seeing:

I modified the Polyline Volume Sandcastle example:

    var blueStar = new Cesium.GeometryInstance({
        geometry : new Cesium.PolylineVolumeGeometry({
            polylinePositions : ellipsoid.cartographicArrayToCartesianArray([
                Cesium.Cartographic.fromDegrees(-95.0, 32.0, 0.0),
                Cesium.Cartographic.fromDegrees(-95.0, 36.0, 100000.0),
                Cesium.Cartographic.fromDegrees(-95.0, 40.0, 100000.0)
            ]),
            vertexFormat : Cesium.PerInstanceColorAppearance.VERTEX_FORMAT,
            shapePositions : starPositions(7, 70000, 50000),
            cornerType : Cesium.CornerType.ROUNDED
        }),
        attributes : {
            color : Cesium.ColorGeometryInstanceAttribute.fromColor(Cesium.Color.BLUE)
        }
    });

It doesn't work for with any vertical segments:

I modified the same example to:

    var blueStar = new Cesium.GeometryInstance({
        geometry : new Cesium.PolylineVolumeGeometry({
            polylinePositions : ellipsoid.cartographicArrayToCartesianArray([
                Cesium.Cartographic.fromDegrees(-95.0, 32.0, 0.0),
                Cesium.Cartographic.fromDegrees(-95.0, 36.0, 100000.0),
                Cesium.Cartographic.fromDegrees(-95.0, 36.0, 200000.0)
            ]),
            vertexFormat : Cesium.PerInstanceColorAppearance.VERTEX_FORMAT,
            shapePositions : starPositions(7, 70000, 50000),
            cornerType : Cesium.CornerType.ROUNDED
        }),
        attributes : {
            color : Cesium.ColorGeometryInstanceAttribute.fromColor(Cesium.Color.BLUE)
        }
    });

You get this if you comment out the first position:

@bagnell alright, this is ready for review

Looks good. Merging.